test: prune low-value tests and parallelize the integration pool - #27
Conversation
The runtime-playground HMR e2e test still stamped a fixed 30s Playwright budget tuned on many-core machines onto waits that sit behind rsbuild compiles and Chrome sharing a two-core runner - the same shape that tripped the capture script on the Node 22.19 Verify job. Both surfaces now read the shared timeScale helper instead of an inline CI multiplier, so the two-core rationale lives in one documented place. Scaling costs nothing on green runs since every wait returns on success.
… and budget the retention test for CI
…and rsc example Removes tests that re-assert coverage pinned elsewhere or assert file contents instead of behavior: the artifacts-real e2e file (its table coverage lives in overview), the 390px Runtime controls e2e, the safe launch configuration overview test (redaction lives in the epoch MCP session test), the handoff close-retry overview test (pinned by runtime-mcp-handoff.test.ts), config/source string-matching tests, and the rsc example's tsconfig/doc/manifest duplicates. Also trims the Inspector-tab detours from the Runtime sibling e2e ahead of the inspector removal.
…suites The Workbench is a desktop-only product validated at 1440x900, so the 390px viewport resizes and horizontal-overflow checks appended to real host e2e tests assert a layout the product does not ship. Removes only those mobile assertion sites, keeps every host test, runs the MCP App preview browser test at the desktop viewport, and deletes the mobile capture path (PNG, mobileLayout evidence, --mobile flag) from the runtime playground capture script plus the README/topology command strings that pin its exact invocation.
Each eviction test prepared its own dev session and drove fifty real invocations before exercising one hook, repeating the slowest setup in the suite four times (and flaking under load). One session now fills the fifty-artifact window once and drives the happy eviction, the held-reader reservation, the failed run-directory removal, and the failed artifact release in eviction order, preserving every distinct assertion including the readRunFlight path-traversal check and the close-retry accounting. Neighbouring retain-until-close, worker-bound, and containment tests are untouched.
…roduction build test The second multi-environment rebuild test repeated the full example build only to check that a planted stale async chunk disappears, and the self-contained widget HTML test duplicated assertions the host artifacts suite already makes per artifact. host-artifacts now plants the stale chunk before its existing production build and asserts its removal, and its HTML artifact loop keeps the inline script/style presence checks, so mcp-transports drops both duplicates.
…e unit pool The three finalization-polling tests exercise readFinalizedEvalRun with in-memory fake clients and never touch a browser or server, yet they lived in evals-real.e2e.test.ts and paid the serialized integration pool for it. They move verbatim to evals-finalized-run.test.ts, which the unit config picks up by default.
…nstead of the default integration pool Every npm pack + clean-install suite (dev-workbench-packaging, packed-consumer, packed-native-smoke, release-audit, rsc-runtime-optional-packaging) leaves the serialized integration pool and moves to a dedicated packedTestFiles list that the unit pool also excludes. The three pack+install cases in public-api.test.ts split into public-api-packed.test.ts so the cheap export and built-entrypoint checks stay in the default loop. test:packed now lists the split file plus rsc-runtime-optional-packaging and packed-native-smoke, keeping per-PR CI coverage through the release-gates job's check:release run.
…service tests Removes tests that only re-assert their own fixtures, string-match documentation, or duplicate a sibling suite: the public-api config/type re-export tests (the built-entrypoint test still pins subpath imports and defineConfig identity), the packed-consumer regex self-test, the manifest re-export identity test, the examples README string test, the topology capture-command README pin, the audiobook tool-catalog and CLI-receipt duplicates, the eval-cli duplicate of the eval-service run path, the release-audit pack dry run (check:release runs pack:dry-run directly), and one redundant JSON.stringify assertion in the native smoke. Type-only contracts previously wrapped in vacuous runtime tests (modern MCP transports, runtime provider binding/surface shapes) stay as module-level @ts-expect-error checks. Deletes the orphaned epoch-atomicity spike (production coverage lives in epoch-store and dev-lock tests) and the micro-eval spot-check suite together with its test:spot-check script and CI job; the examples:check step from that job survives as its own examples-check job.
…-metadata rehash test adapter-metadata.test.ts already rehashes every capability and schema snapshot against its pinned provenance for all built-in targets, so the host-adapters CLI-version test keeps only its unique assertions: the observed CLI version pins, the redacted help text, and the codex marketplace validator fixture.
…ifact writers into a serial pool The parallel pool runs mkdtemp-fixture, ephemeral-port files on min(4, cores/2) workers (still 1 on two-core CI, overridable via AGENT_BUNDLE_INTEGRATION_MAX_WORKERS); the six files that rewrite shared package dists stay on one worker in a chained serial config.
overview.e2e, mcp-app-real.e2e, and playground-real.e2e now use the shared memoized buildWorkbench (which honors AGENT_BUNDLE_WORKBENCH_PREBUILT) instead of local copies that rebuilt packages/workbench/dist up to ten times per run; cli.test and the packed-release harness honor a new AGENT_BUNDLE_PACKAGE_PREBUILT flag, with `test:integration` building the whole workspace once up front. With those writes gone, the four files move from the serial pool to the parallel one.
…o the eviction suite The session accepts a test-only maximumRunHistory through its start testing seam; the consolidated eviction test drives a five-run window instead of fifty real invocations and asserts the production default is still fifty.
…rallel workers Concurrent Chrome + dev-server + rsbuild pairs contend for cores, so the parallel config doubles timeScale for its workers and the e2e suites with previously unscaled budgets now honor it (same mechanism CI already uses on two-core runners).
Real in-process example builds can exceed the 5s rstest default when four workers share the machine; explicit per-test timeouts are unaffected.
…gration-suite # Conflicts: # packages/workbench/tests/overview.e2e.test.ts # packages/workbench/tests/runtime-playground.e2e.test.ts
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8542a85356
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Heads-up from the sibling session: the four failing checks here are all starvation-shaped — the packed dogfood epoch mismatch, the capture-script failure on Node 26, and the heartbeat-lock timing test are the same class of 2-core-runner budget exhaustion that #20 root-caused, and the pkg.pr.new failure is an external 500. Parallelizing the integration pool on 2-core CI runners may be reintroducing the oversubscription that the serialized pool avoided; if the rerun fails the same way, consider gating the parallel pool's worker count on CI (e.g. keep maxWorkers 1 when CI is set, parallel locally). |
Hosted runners report 4 cores, so the half-cores formula computed 2 workers there. Each Chrome + dev-server + rsbuild pair already saturates the runner, and 2-worker matrix legs failed on a rotating contention timeout per run even at timeScale 4. CI keeps the serialized shape its budgets were tuned for; the parallel speedup remains a development-machine feature. AGENT_BUNDLE_INTEGRATION_MAX_WORKERS still overrides both directions.
- close the fixture-server listener before destroying held connections so a reconnecting browser cannot slip a new request past teardown (#24) - honor AGENT_BUNDLE_PACKAGE_PREBUILT in public-api.test.ts so the parallel integration pool never rewrites the shared dist directories (#27) - move packed-release.e2e into the packed suite so `pnpm test` and `check:release` stop running the same long packed-browser suite twice, retiring the now-empty serial integration pool (#22) - reconcile the fidelity ledger with the desktop-only capture interface and drop the retired mobile.png evidence (#22)
Supersedes #25 (which was merged into the now-dead staging branch
test/prune-low-value-testsinstead of main) and carries the content of #22 (which was likewise merged into the abandonedfix/deflake-root-causesbranch). Neither stage ever reached main; this PR lands both.What this PR delivers
rstest.integration-tests.ts).pool.maxWorkers, a serial pool for shared-artifact writers, prebuilt-artifact reuse (AGENT_BUNDLE_WORKBENCH_PREBUILT/AGENT_BUNDLE_PACKAGE_PREBUILT), an injectable run-history retention window for the eviction suite, and contention hardening (timeScalebudget scaling + 30s parallel default timeout).origin/mainhas been merged in (inspector removal from #21, the overview budget commit, and the fixture-server connection fix from #24). Conflict resolutions kept the pruned tests deleted while adopting main's edits to surviving tests (inspector-tab assertion removals, the renamed runtime-MCP-routing test,timeScalebudget scaling). The serial pool now contains onlypacked-release.e2e—inspector-shell.e2ewas deleted on main along with the vendored inspector, so its manifest entry is dropped.Parallelization details
rstest.integration.config.tscomputespool.maxWorkersasmax(1, min(4, floor(availableParallelism() / 2))). CI pins one worker: hosted runners report 4 cores (computing 2 workers), but each Chrome + dev-server + rsbuild pair already saturates them — 2-worker matrix legs failed on a rotating contention timeout per run even at timeScale 4.AGENT_BUNDLE_INTEGRATION_MAX_WORKERSoverrides both directions.pool/isolate, so the split is two chained configs: the parallel pool runs files that only usemkdtempfixtures, ephemeral ports, and read-only prebuilt dists;rstest.integration-serial.config.tsthen runspacked-release.e2e(root build fallback +npm pack+ pre-reserved port). Per-file module isolation is preserved for the log pipeline suites.overview.e2e(up to 10 builds/run),mcp-app-real.e2e(2), andplayground-real.e2ehad localbuildWorkbenchcopies that ignoredAGENT_BUNDLE_WORKBENCH_PREBUILT; they now use the shared memoized helper.cli.testand the packed-release harness honorAGENT_BUNDLE_PACKAGE_PREBUILT, withtest:integrationrunning one rootpnpm buildup front. This part speeds CI up too: the serialized Verify leg dropped from ~16 min to ~10 min.maximumRunHistoryseam; the consolidated eviction test drives a 5-run window instead of 50 real invocations and asserts the production default is still 50.timeScalepolling budgets and the parallel pool raises the 5s default test timeout to 30s; explicit per-test timeouts win.Measured times (96-core machine, background load avg ~22-25 from other jobs)
AGENT_BUNDLE_INTEGRATION_MAX_WORKERS=1, CI shape)pnpm test:unitTest plan (re-verified after the main merges)
pnpm build+ workbench build,pnpm typecheck,pnpm lint— cleanpnpm test:unit— green (all tests pass)overview.e2e,runtime-playground.e2e) against prebuilt artifacts — green, including the merged runtime-MCP-routing testpnpm test:integration:runparallel (4 workers) — 525+/528 per pass in ~4 min; remaining failures are single rotating contention flakes in files this PR does not touch (each passes in isolation), which is why CI runs the pool serialized